Added tests and documentation for cases zero neighbor cases. #775
Conversation
Codecov Report
@@ Coverage Diff @@
## master #775 +/- ##
==========================================
+ Coverage 55.38% 55.48% +0.09%
==========================================
Files 16 16
Lines 2580 2581 +1
Branches 38 38
==========================================
+ Hits 1429 1432 +3
+ Misses 1135 1133 -2
Partials 16 16
Continue to review full report at Codecov.
|
cpp/order/HexaticTranslational.cc
Outdated
| m_psi_array[i] /= std::complex<float>(m_k); | ||
| if (total_weight == 0.) | ||
| { | ||
| m_psi_array[i] /= std::complex<float>(total_weight); |
There was a problem hiding this comment.
Force this to be nan by dividing by zero. Is there a better way to do this? I saw std::nanf but I wasn't sure what to pick as the argument.
There was a problem hiding this comment.
I haven't reviewed this PR thoroughly yet, but I'm not sure this is generating the output you want. std::complex<float>(1)/std::complex<float>(0) will return (inf, nan), not just nan (here's an example)
There was a problem hiding this comment.
So tried to set m_psy_array equal to just nan like this:
m_psi_array[i] = std::complex<float>(std::nanf)and then like this
m_psi_array[i] = std::complex<float>(std::nanf)All tests pass on my laptop, but fail on CI, so I reverted back to a divide by zero. I double checked the values from the no neighbor test for the Translational order parameter, and in python, it registers as nan and not inf.
|
@klywang any updates on this? |
for more information, see https://pre-commit.ci
…b/freud into feature/zero-neighbors
We checked the behavior of Hexatic, Translational, SolidLiquid, and LocalDescriptors handle cases with zero neighbors.
For particles without neighbors:
nanforwl=Trueandwl=Falsenanparticle_harmonicsreturnsnannum_sphsreturns 0,sph=[]Description
Added tests for edge cases and updated docs where
nanis expected.TODO: Had some issue building the docs. Most of the notes did not show up.
Motivation and Context
Resolves: #678
How Has This Been Tested?
Added unit tests.
Screenshots
Types of changes
Checklist: